Skip to content

REPL: warn when a non-last module-level expression result is discarded (BT-979)#1022

Merged
jamesc merged 1 commit intomainfrom
worktree-BT-979
Mar 1, 2026
Merged

REPL: warn when a non-last module-level expression result is discarded (BT-979)#1022
jamesc merged 1 commit intomainfrom
worktree-BT-979

Conversation

@jamesc
Copy link
Owner

@jamesc jamesc commented Feb 28, 2026

Summary

  • Enable the effect-free statement lint on module.expressions by default, so REPL users see warnings when expression results are silently discarded (e.g., #{#x => 1} #{#y => 2} where the first map literal is discarded)
  • Add skip_module_expression_lint flag to CompilerOptions so bootstrap-test compilation can opt out (test fixtures intentionally use top-level expressions with // => assertions)
  • Expand REPL warning filter to include Severity::Lint diagnostics

Linear issue: https://linear.app/beamtalk/issue/BT-979

Key Changes

  • CompilerOptions gains skip_module_expression_lint: bool (default false)
  • check_effect_free_statements now checks module.expressions unless the flag is set
  • REPL compiler port surfaces Severity::Lint warnings to users
  • Bootstrap-test compilation sets the flag to true to suppress false positives
  • 4 new tests, 1 updated test, all 4,838 tests pass

Test plan

  • Existing effect-free lint tests pass
  • New module-level lint tests verify detection and opt-out
  • Bootstrap-test compilation doesn't produce false positives
  • Full CI passes (just ci)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Improvements
    • REPL now displays linting hints for effect-free statements, providing enhanced feedback during interactive development.
    • Adjusted compiler linting behavior for module-level expressions in test fixtures.

…BT-979

- Enable effect-free lint check on module.expressions by default
- Add skip_module_expression_lint flag to CompilerOptions for bootstrap-test opt-out
- Include Severity::Lint in REPL compiler port warnings so users see the hint
- Add tests for module-level expression linting (default-on and opt-out)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Feb 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 213f695 and 28bacd6.

📒 Files selected for processing (7)
  • crates/beamtalk-cli/src/commands/test_stdlib.rs
  • crates/beamtalk-cli/src/main.rs
  • crates/beamtalk-compiler-port/src/main.rs
  • crates/beamtalk-core/src/lib.rs
  • crates/beamtalk-core/src/lint/effect_free_statement.rs
  • crates/beamtalk-core/src/semantic_analysis/mod.rs
  • crates/beamtalk-core/src/semantic_analysis/validators.rs

📝 Walkthrough

Walkthrough

Adds a new skip_module_expression_lint boolean field to CompilerOptions to conditionally suppress linting of top-level module expressions. The flag is threaded through the semantic analysis pipeline and set to true during test fixture compilation while defaulting to false elsewhere, allowing effect-free statement checks on module-level expressions to be opted out.

Changes

Cohort / File(s) Summary
CompilerOptions Struct Definition
crates/beamtalk-core/src/lib.rs
Added public field skip_module_expression_lint: bool to control effect-free lint checks on module expressions (defaults to false).
Compilation Options Initialization
crates/beamtalk-cli/src/main.rs, crates/beamtalk-compiler-port/src/main.rs
Updated CompilerOptions initialization to use ..Default::default() pattern and wired skip_module_expression_lint flag from options through compilation pipeline.
Effect-Free Statement Linting
crates/beamtalk-core/src/semantic_analysis/validators.rs, crates/beamtalk-core/src/lint/effect_free_statement.rs
Updated check_effect_free_statements() signature to accept skip_module_expression_lint boolean parameter; added tests verifying module-level expression linting behavior with and without the flag.
Semantic Analysis Pipeline
crates/beamtalk-core/src/semantic_analysis/mod.rs
Threaded skip_module_expression_lint flag through internal analyse_full() function and updated callers (analyse_with_options) to propagate the option from CompilerOptions.
Test Fixture Compilation
crates/beamtalk-cli/src/commands/test_stdlib.rs
Set skip_module_expression_lint: true when compiling test fixtures to suppress module-level expression lint checks (BT-979).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • #990 — Introduces the effect-free-statement lint that this PR extends with conditional skipping of module-expression checks.
  • #994 — Refactors module expression traversal in validators.rs, directly affecting the same code paths modified here.
  • #992 — Modifies the semantic analysis pipeline entry point (analyse_full) to add a separate lint pass, sharing infrastructure with this PR's analysis-level changes.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: enabling linting to warn about discarded non-last module-level expressions in the REPL.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch worktree-BT-979

Comment @coderabbitai help to get the list of available commands and usage tips.

@jamesc jamesc merged commit 38aef01 into main Mar 1, 2026
5 checks passed
@jamesc jamesc deleted the worktree-BT-979 branch March 1, 2026 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant